chore: unify configuration properties - #2545
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes service/model configuration property names under ai.timefold.model.* (user-defined) and ai.timefold.platform.* (platform-injected), including renaming the former timefold.application.version to ai.timefold.model.api-version and updating all in-repo usages touched by the service modules.
Changes:
- Renamed injected/configured property keys across worker, Quarkus deployment/runtime, maps client, and test modules to the new
ai.timefold.model.*namespace (and kept platform-specific keys underai.timefold.platform.*). - Updated default
application.propertiestemplates to useai.timefold.model.*placeholders (notably REST base path viaai.timefold.model.api-version). - Updated Maven property names and build flags (
.mvn/maven.config, service parent POM) to match the new configuration keys.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| service/worker/src/main/java/ai/timefold/solver/service/worker/impl/termination/TerminationConfigParams.java | Renames termination property keys to ai.timefold.model.termination.*. |
| service/worker/src/main/java/ai/timefold/solver/service/worker/impl/SolverWorker.java | Switches injected model name/version properties to ai.timefold.model.*. |
| service/worker/src/main/java/ai/timefold/solver/service/worker/impl/metrics/PlatformMetricsMeterFilter.java | Switches injected model identifier property to ai.timefold.model.id. |
| service/tools/maven-plugin/README.adoc | Documents the new ai.timefold.model.api-version key for model registration/version validation. |
| service/test/src/main/java/ai/timefold/solver/service/test/impl/quarkus/ModelRequestSerializationCallback.java | Renames test serialization toggle property to ai.timefold.model.* and fixes wording. |
| service/test-model/src/test/java/ai/timefold/solver/model/testmodel/OpenApiTest.java | Updates injected API version property key. |
| service/test-model/src/test/java/ai/timefold/solver/model/testmodel/EmployeeScheduleResourceTest.java | Updates injected API version property key. |
| service/test-model/src/test/java/ai/timefold/solver/model/testmodel/ApiVersion2ConfigProfile.java | Updates overridden API version property key. |
| service/test-model/src/main/resources/application.properties | Renames model metadata + test termination config keys to ai.timefold.model.*. |
| service/service-defaults/src/main/resources/application.properties | Updates model metadata placeholders and REST/OpenAPI/container naming to ai.timefold.model.*. |
| service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/rest/schema/ValidatingReaderInterceptorContext.java | Renames schema validation enablement property to ai.timefold.model.schema.validation.enable. |
| service/quarkus/integration-tests/src/main/resources/application.properties | Updates integration-test config to ai.timefold.model.*. |
| service/quarkus/deployment/src/test/java/ai/timefold/solver/service/quarkus/deployment/VisualizationPagesConfigTest.java | Updates config keys and expected bind failure messages to the new prefix. |
| service/quarkus/deployment/src/test/java/ai/timefold/solver/service/quarkus/deployment/ApplicationVersionValidationTest.java | Updates expected error message content for renamed API version key. |
| service/quarkus/deployment/src/main/java/ai/timefold/solver/service/quarkus/deployment/TimefoldModelDescriptorProcessor.java | Renames all model descriptor metadata property keys and updates validation messaging. |
| service/quarkus/deployment/src/main/java/ai/timefold/solver/service/quarkus/deployment/config/VisualizationPagesConfig.java | Renames @ConfigMapping prefix to ai.timefold.model.visualization. |
| service/maps/service-client/src/main/java/ai/timefold/solver/service/maps/service/client/impl/MapServiceOptionsSupplier.java | Updates injected model id/version/resource property keys. |
| service/facade/service-parent/pom.xml | Renames Maven properties (including api-version) and Quarkus plugin propagated properties. |
| .mvn/maven.config | Renames the build flag property to ai.timefold.model.processing.disabled. |
| ######################## | ||
| # REST API configuration | ||
| ######################## | ||
| quarkus.rest.path=${timefold.application.version} | ||
| quarkus.rest.path=${ai.timefold.model.api-version} | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
service/service-defaults/src/main/resources/application.properties:86
- Docs in docs/src/modules/ROOT still reference the renamed properties (for example timefold.application.* and ai.timefold.platform.termination.*), so they will be incorrect after this change unless updated in the same release.
########################
# REST API configuration
########################
quarkus.rest.path=${ai.timefold.model.api-version}
diogodanielsoaresferreira
left a comment
diogodanielsoaresferreira
left a comment
There was a problem hiding this comment.
Also added an issue for the platform to sync with the new properties when we update to this solver version https://github.com/TimefoldAI/timefold-platform/issues/5238
| "ai.timefold.model.termination.spent-limit", | ||
| "ai.timefold.model.termination.maximum-spent-limit", | ||
| "ai.timefold.model.termination.unimproved-spent-limit", | ||
| "ai.timefold.model.termination.maximum-unimproved-spent-limit", |
There was a problem hiding this comment.
This is also configured in the platform. Do we want to keep them with a model prefix?
There was a problem hiding this comment.
That's the problem. Many properties are injected by the platform, yet they are configured in application.properties as well, typically for local runs and testing. Termination is a typical example of that, see e.g. https://github.com/TimefoldAI/timefold-employee-scheduling/blob/main/src/main/resources/application.properties#L50-L74.
Not all of them can be solved by good defaults. Some we even describe in the documentation.
Most of them are actually settings of the model, just take different values in the platform and locally.
There was a problem hiding this comment.
I'm ok with that 👍
|
winklerm
left a comment
winklerm
left a comment
There was a problem hiding this comment.
Thank you, looks good to me!
@rsynek @diogodanielsoaresferreira I have one concern that I would like to double check:
- After the properties are renamed, the models using the new solver service will be compatible only with the platform using the new solver service as well. That is business as usual.
- However, also the other way around applies: all older models will stop working with the new platform. This means we will have to upgrade and release new versions of all older models used in staggered releases for selected customers and upgrade all at once.
Is my understanding correct?
Yes, technically that is correct. We can try to support old and new properties in the platform for a while, so that the models do not break. |
Fixes #2496.
Renames configuration properties by the following logic:
Some duplicate properties removed,
timefold.application.versionrenamed toai.timefold.model.api-versionto better reflect what it is.Related: https://github.com/TimefoldAI/timefold-solver-enterprise/pull/795
TODO: